feat(cli): honest non-interactive init — fail on skew, no false success (rc.2 M4)#687
feat(cli): honest non-interactive init — fail on skew, no false success (rc.2 M4)#687coderdan wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 1d245a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthrough
ChangesInit outcome handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant initCommand
participant installDepsStep
participant installEqlStep
participant AgentHandoff
User->>initCommand: run stash init
initCommand->>installDepsStep: install dependencies
installDepsStep-->>initCommand: dependency state or CliExit
initCommand->>installEqlStep: install EQL extension
installEqlStep-->>initCommand: eqlInstalled state
initCommand->>AgentHandoff: build prompt from installed skills
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/impl/steps/handoff-claude.ts`:
- Around line 41-48: Make all generated handoff guidance respect the
installed-skills state: in handoff-claude.ts, update the setup prompt loaded by
the launchPrompt flow so the empty-skills variant does not reference skill
directories or the unwritten AGENTS.md file, while retaining valid skill
guidance when skills exist; in handoff-codex.ts, update its generated guidance
so .codex/skills/ is omitted when installed.length is zero.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8da61ce-6475-4788-b2d3-6295b53bb730
📒 Files selected for processing (9)
.changeset/honest-noninteractive-init.mdpackages/cli/src/commands/impl/steps/handoff-claude.tspackages/cli/src/commands/impl/steps/handoff-codex.tspackages/cli/src/commands/init/__tests__/init-command.test.tspackages/cli/src/commands/init/index.tspackages/cli/src/commands/init/steps/__tests__/install-deps.test.tspackages/cli/src/commands/init/steps/install-deps.tspackages/cli/src/messages.tsskills/stash-cli/SKILL.md
…ss (rc.2 M4) Non-interactive `stash init` reported success for setups that didn't fully complete. Three honesty gaps closed: 1. Version skew. A non-interactive run can't reconcile a `behind` skew (it won't mutate an install without consent), so instead of warning-and- proceeding — scaffolding against packages older than this CLI expects and then claiming success — it now REFUSES with a non-zero exit (CliExit(1)) and the exact align command. The #661/#666 no-mutation principle is preserved (execSync still not called); only the post-warning action changes from proceed to fail. `ahead` skew stays a warn (install likely fine; update the CLI). 2. False 'Setup complete'. The summary header and the auth/db/scaffold checkmarks were unconditional. Now: when EQL is required but not installed (eqlInstalled=false AND integration !== prisma-next), the summary reads 'Setup incomplete' and init exits non-zero, pointing at `stash eql install`. 'Database connection verified' → 'Database URL resolved' (init resolves a URL, never opens a connection). 'Encryption client scaffolded' is shown only when a client was written (clientFilePath set; skipped for prisma-next). 3. False 'skills loaded'. The Claude/Codex handoff launch prompt referenced the skills dir unconditionally; a stripped build installs no skills, so it told the agent to read files that aren't there. The clause is now conditional on installSkills() actually copying something. Tests: install-deps skew tests updated to assert the refusal (still never mutates); new init-command honest-summary tests (EQL-missing → exit 1 + 'Setup incomplete'; prisma-next eqlInstalled=false → completes). Skill + doc comments updated. Suite 542 unit / 62 e2e green, code:check clean. Changeset: stash minor. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
27520d6 to
fb77e14
Compare
There was a problem hiding this comment.
Pull request overview
This PR makes stash init reliable for automation by ensuring non-interactive runs never report success when setup is incomplete or potentially inconsistent with the CLI’s expected runtime package versions.
Changes:
- Make non-interactive init refuse (exit non-zero) on behind version skew instead of warning-and-proceeding.
- Make the init summary truthful: “Database URL resolved” (not “connection verified”), “Setup incomplete” + exit non-zero when EQL is missing (except Prisma Next), and only reference skills when they were actually installed.
- Update unit coverage, changeset, and the published
stash-cliskill documentation to match the new behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| skills/stash-cli/SKILL.md | Documents the new “fail on behind skew” and “setup incomplete” non-interactive behavior. |
| packages/cli/src/messages.ts | Adds stable message leaders for non-interactive init failure modes. |
| packages/cli/src/commands/init/steps/install-deps.ts | Refuses non-interactively on behind skew (throws CliExit(1)), while keeping ahead skew as warn-and-proceed. |
| packages/cli/src/commands/init/steps/tests/install-deps.test.ts | Updates skew tests to assert refusal + no mutation in non-interactive mode. |
| packages/cli/src/commands/init/index.ts | Adjusts checkmark copy, adds EQL-missing “Setup incomplete” gate + non-zero exit (except Prisma Next). |
| packages/cli/src/commands/init/tests/init-command.test.ts | Adds unit tests asserting the honest-summary behavior and Prisma Next exception. |
| packages/cli/src/commands/impl/steps/handoff-codex.ts | Makes the “skills” clause in the launch prompt conditional on actual skill installation. |
| packages/cli/src/commands/impl/steps/handoff-claude.ts | Makes the “skills” clause in the launch prompt conditional on actual skill installation. |
| .changeset/honest-noninteractive-init.md | Adds release notes describing the non-interactive honesty changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two "only claim what actually happened" gaps found in review: - Copilot: the "✓ Encryption client scaffolded" checkmark was gated on `state.clientFilePath`, which build-schema sets even when it KEEPS an existing file (writes nothing). Gate on `state.schemaGenerated` (true only when a placeholder was written); show "✓ Encryption client kept (existing file)" for the keep path. Prisma Next (no clientFilePath) still shows neither. - CodeRabbit: the setup-prompt "Skills loaded" section pointed at skill directories and `AGENTS.md` regardless of what the handoff wrote — so a stripped build (no skills) sent the agent to read files that don't exist, and the Claude handoff (which never writes AGENTS.md) named it as the doctrine source. New `skillsLoadedLines(handoff, installedSkills)` renders the section honestly: no skills → point only at what was written (AGENTS.md for codex/agents-md, the docs for claude-code); claude-code → doctrine is in the installed skills, not AGENTS.md. Also hedged "list the skills loaded (if any)". Added setup-prompt honesty tests (empty-skills × handoff) and de-duped the two identical inline blocks. 557 tests green; build + biome clean. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Problem (rc.2 M4)
Non-interactive
stash initreported success for setups that hadn't fully completed — so CI/agents couldn't trust its exit code or its summary. Three honesty gaps:behindskew (an installed@cipherstash/*older than this CLI expects) — it won't mutate an install without consent — so it warned and kept going, scaffolding config/client against mismatched packages, then printed "Setup complete".eqlInstalled=false), the header still said complete — and "✓ Database connection verified" was printed although init only resolves a URL, never connects.installSkills()copied nothing (stripped build) — telling it to read files that aren't there.Fix
behindskew → refuse with a non-zero exit (CliExit(1)) and the exact align command, instead of proceeding. The npm dist-tags deliver broken/stale packages: adapters' latest=0.0.0, stack latest=0.19.0; stash init installs unpinned #661/fix(cli): pin init's installs to the release's own package versions (#661) #666 no-mutation principle is preserved (execSyncstill never called — only the post-warning action changes).aheadskew stays a warn (install likely fine; update the CLI). Interactive runs still offer to align.!eqlInstalled && integration !== 'prisma-next'), the summary reads "Setup incomplete" and init exits non-zero, pointing atstash eql install. "Database connection verified" → "Database URL resolved"; "Encryption client scaffolded" shows only when a client was actually written (skipped for Prisma Next).installSkills()returning something.Scope notes
prisma-next migration apply, soeqlInstalled=falseis expected there and is NOT treated as incomplete (tested).Verification
install-depsskew tests (refuses + exit 1, still never mutates); newinit-commandhonest-summary tests (EQL-missing → exit 1 + "Setup incomplete"; prisma-nexteqlInstalled=false→ completes).code:checkclean. Changeset:stashminor. Skill (stash-cli) + doc comments updated.initneeds live auth + a DB (which is why there's no init e2e); the honest-summary and skew-fail logic is unit-covered.https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Summary by CodeRabbit
Bug Fixes
stash initnow exits with an error when required dependencies have incompatible versions and provides an alignment command.Documentation
stash initguidance for dependency version mismatches and incomplete EQL setup.